Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kbn/optimizer] implement more efficient auto transpilation for node #79052

Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Oct 1, 2020

Blocked by #78168
Probably fixes: #79052

Experimental reimplementation of @babel/register using lmdb-store as the cache instead of a simple object in memory. lmdb-store provides synchronous access to values stored in a memory-mapped file and does writes batched and asynchronously. This makes it a really nice fit for this use-case and allows us to keep jit-transpilation without the cost of loading the entire cache into memory or writing it all back to disk at once.

Additionally, the cache implementation in this PR keeps caches from multiple babel configurations (like you might experience on different branches of Kibana) and automatically prunes values from the cache after they haven't been accessed for 30 days.

Stats:
all these measurements were taken with optimizer bundles fully cached and using the default --max-old-space-size (without #78710 merged)

master:
time to server listening, no cache: 91 seconds
time to server listening, with cache: 40 seconds
memory of parent process, no cache: ~850mb
idle memory of parent process, with cache: 760mb
idle memory of server process, with cache: 1.1gb

this pr:
time to server listening, no cache: 63 seconds
time to server listening, with cache: 16 seconds
memory of parent process, no cache: ~650mb
idle memory of parent process, with cache: 300mb
idle memory of server process, with cache: 320mb

@spalger spalger added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Oct 1, 2020
@spalger spalger force-pushed the implement/kbn-optimizer-auto-transpile branch 2 times, most recently from 4682ae7 to 6d25fa0 Compare October 1, 2020 15:24
…-optimizer-auto-transpile

# Conflicts:
#	yarn.lock
@spalger spalger marked this pull request as ready for review October 2, 2020 01:39
@spalger spalger requested review from a team as code owners October 2, 2020 01:39
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Oct 2, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@mistic
Copy link
Member

mistic commented Oct 2, 2020

@spalger as this introduces a native module dependency for the kbn optimizer it will cause problems with the bazel remote cache mechanism. Do you think we can fork lmdb-store, simply integrate it with https://github.com/prebuild/prebuildify so we can ship all the prebuilts with the npm publish of the module? Looking at the module the change should be really simple, just add prebuildify as a devDependency, add a script step to run it and then call that prebuildify script in the publish script. (for example https://github.com/parcel-bundler/watcher/blob/master/package.json#L28)

@spalger
Copy link
Contributor Author

spalger commented Oct 2, 2020

@mistic Do you mind if we work on this after merging? I'd like to suggest using prebuildify to the lmdb-store devs rather than forking and maintaining a fork, but I need to learn about how that works first.

@mistic
Copy link
Member

mistic commented Oct 2, 2020

@spalger that is totally okay! Just wanted to raise a flag here so we can take care of it sooner rather than later 😃

// for a much more detailed explanation
require('./register');
require('./polyfill');
export * from './node_auto_tranpilation';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spalger why don't we create a new package called @kbn/node-env or just @kbn/node in order to store that logic plus the one we have on src/setup_node_env? Do you think that will be to much and we should just add it to the kbn/optimizer extending its boundaries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be too little to justify a new package personally, and it fits into what I had imagined @kbn/optimizer would become. I also plan to move the rest of setup_node_env into @kbn/optimizer/node but just taking the simplest path for now.

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/ui-shared-deps asset size

id before after diff
kbn-ui-shared-deps.js 5.2MB 5.2MB +21.0B

async chunks size

id before after diff
enterpriseSearch 430.2KB 430.2KB -17.0B

distributable file count

id before after diff
default 43965 41240 -2725
oss 24624 24863 +239

page load bundle size

id before after diff
beatsManagement 439.1KB 439.8KB +723.0B
ingestManager 510.5KB 511.3KB +723.0B
upgradeAssistant 64.9KB 64.8KB -17.0B
total +1.4KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@spalger spalger merged commit ddf2d82 into elastic:master Oct 3, 2020
@spalger spalger deleted the implement/kbn-optimizer-auto-transpile branch October 3, 2020 01:36
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 5, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 79052 or prevent reminders by adding the backport:skip label.

spalger pushed a commit to spalger/kibana that referenced this pull request Oct 5, 2020
…lastic#79052)

Co-authored-by: spalger <[email protected]>
# Conflicts:
#	packages/kbn-optimizer/package.json
#	src/dev/build/tasks/create_empty_dirs_and_files_task.ts
#	src/setup_node_env/babel_register/register.js
#	yarn.lock
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 5, 2020
spalger added a commit that referenced this pull request Oct 5, 2020
… node (#79052) (#79395) (#79408)

* [kbn/optimizer] implement more efficient auto transpilation for node (#79052)

Co-authored-by: spalger <[email protected]>
# Conflicts:
#	packages/kbn-optimizer/package.json
#	src/dev/build/tasks/create_empty_dirs_and_files_task.ts
#	src/setup_node_env/babel_register/register.js
#	yarn.lock

* fix eslint violation

* add core-js production dependency (#79395)

Co-authored-by: spalger <[email protected]>
(cherry picked from commit 9e1c44f)

Co-authored-by: spalger <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 5, 2020
* master: (128 commits)
  add core-js production dependency (elastic#79395)
  Add support for sharing saved objects to all spaces (elastic#76132)
  [Alerting UI] Display a banner to users when some alerts have failures, added alert statuses column and filters (elastic#79038)
  load js-yaml lazily (elastic#79092)
  skip flaky suite (elastic#77278)
  Fix agentPolicyUpdateEventHandler() to use app context soClient for creation of actions (elastic#79341)
  [Security Solution] Untitled Timeline created when first action is to add note (elastic#78988)
  [Security Solutions][Detection Engine] Updates the edit rules page to:wq! only have what is selected for editing (elastic#79233)
  Cleanup yarn.lock from duplicates (elastic#66617)
  [kbn/optimizer] implement more efficient auto transpilation for node (elastic#79052)
  [Ingest Manager] Rename Fleet setup and requirement, Fleet => Central… (elastic#79291)
  [core/server/plugins] don't run discovery in dev server parent process (take 2) (elastic#79358)
  [babel/register] remove from build (take 2) (elastic#79379)
  [Security Solution] Changes rules table tag display (elastic#77102)
  define integrationTestRoot in config file and use to define screensho… (elastic#79247)
  Revert "[babel/register] remove from build (elastic#79176)"
  skip flaky suite (elastic#75241)
  [Uptime] Synthetics UI (elastic#77960)
  [Security Solution] [Detections] Only display actions options if user has "read" privileges (elastic#78812)
  [babel/register] remove from build (elastic#79176)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 5, 2020
* master: (288 commits)
  add core-js production dependency (elastic#79395)
  Add support for sharing saved objects to all spaces (elastic#76132)
  [Alerting UI] Display a banner to users when some alerts have failures, added alert statuses column and filters (elastic#79038)
  load js-yaml lazily (elastic#79092)
  skip flaky suite (elastic#77278)
  Fix agentPolicyUpdateEventHandler() to use app context soClient for creation of actions (elastic#79341)
  [Security Solution] Untitled Timeline created when first action is to add note (elastic#78988)
  [Security Solutions][Detection Engine] Updates the edit rules page to only have what is selected for editing (elastic#79233)
  Cleanup yarn.lock from duplicates (elastic#66617)
  [kbn/optimizer] implement more efficient auto transpilation for node (elastic#79052)
  [Ingest Manager] Rename Fleet setup and requirement, Fleet => Central… (elastic#79291)
  [core/server/plugins] don't run discovery in dev server parent process (take 2) (elastic#79358)
  [babel/register] remove from build (take 2) (elastic#79379)
  [Security Solution] Changes rules table tag display (elastic#77102)
  define integrationTestRoot in config file and use to define screensho… (elastic#79247)
  Revert "[babel/register] remove from build (elastic#79176)"
  skip flaky suite (elastic#75241)
  [Uptime] Synthetics UI (elastic#77960)
  [Security Solution] [Detections] Only display actions options if user has "read" privileges (elastic#78812)
  [babel/register] remove from build (elastic#79176)
  ...
@jinmu03 jinmu03 mentioned this pull request Oct 5, 2020
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Operations Team label for Operations Team v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants